Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

update esm4p2 and null model yamls for c5 update #270

Merged
merged 2 commits into from
Nov 26, 2024
Merged

update esm4p2 and null model yamls for c5 update #270

merged 2 commits into from
Nov 26, 2024

Conversation

rem1776
Copy link
Contributor

@rem1776 rem1776 commented Nov 15, 2024

Describe your changes

just updates the esm4p2 and null model example yamls to work with the latest c5 modules

Checklist before requesting a review

  • I ran my code
  • I tried to make my code readable
  • I tried to comment my code
  • I wrote a new test, if applicable
  • I wrote new instructions/documentation, if applicable
  • I ran pytest and inspected it's output
  • I ran pylint and attempted to implement some of it's feedback

singhd789
singhd789 reviewed Nov 15, 2024
View reviewed changes
Copy link
Collaborator

@singhd789 singhd789 left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for updating this Ryan! I just have a few clarifying questions. Also, do you think we should keep JUST the null model example in here for testing, and move the AM5 and ESM4p2 completely over to fre-examples? I think this would make it slightly less convoluted here.

fre/make/tests/ESM4_example/compile.yaml Outdated Show resolved Hide resolved
fre/make/tests/ESM4_example/platforms.yaml Show resolved Hide resolved
@rem1776
Copy link
Contributor Author

rem1776 commented Nov 15, 2024

Thanks for updating this Ryan! I just have a few clarifying questions. Also, do you think we should keep JUST the null model example in here for testing, and move the AM5 and ESM4p2 completely over to fre-examples? I think this would make it slightly less convoluted here.

Yeah I think thats a great idea, I'll remove the directories with this PR and make a new one to fre-examples.

@rem1776
Copy link
Contributor Author

rem1776 commented Nov 15, 2024 •
edited by singhd789
Loading

@singhd789 I think a couple tests use the am5.yaml so this will break the testing. Maybe we should keep those around for now, unless we want to go crazy and add another git submodule for the fre-examples repo lol.

@singhd789
Copy link
Collaborator

singhd789 commented Nov 15, 2024 •
edited
Loading

@singhd789 I think a couple tests use the am5.yaml so this will break the testing. Maybe we should keep those around for now, unless we want to go crazy and add another git submodule for the fre-examples repo lol.

Oh interesting. I think you may be right (fre cli make tests maybe). Now I'm wondering how the checks passed without those. We can keep the AM5 yamls in for now if needed (we should change those to use the null model). Good catch!

@singhd789 singhd789 merged commit 5775bab into NOAA-GFDL:main Nov 26, 2024
1 check passed
@ilaflott
Copy link
Member

Odd, it looks like the second check never actually ran.

@singhd789
Copy link
Collaborator

singhd789 commented Nov 26, 2024 •
edited
Loading

Odd, it looks like the second check never actually ran.

I noticed that after I hit merge that "1 check passed" and I thought that was weird. Briefly looking at the action failure though, it looks related to another PR I have open (#275) - how the compile log wasn't actually being made

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@singhd789 singhd789 singhd789 approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@rem1776 @singhd789 @ilaflott